Review: PR #232

Thanks for this PR — great initiative! The two-tier routing design (static name matching + package.json detection) is clean and the FilterRoute enum is well-structured. I checked out the branch, built the release binary, and tested against a real pnpm project. Here are the findings.

Local test results (14 commands)

Comparison — rtk vitest run (existing filter) vs rtk pnpm run test (this PR):

$ rtk vitest run          → PASS (2) FAIL (0)          # ✅ compact
$ rtk pnpm run test       → 📊 OUTPUT (last 5 lines):  # ❌ raw fallback
                              ✓ src/index.test.ts (2 tests) 2ms
                              Test Files  1 passed (1)
                              ...

The main selling point of this PR (smart routing to vitest/playwright filters) doesn't work in practice.

Bugs to fix

1. Vitest/Playwright filter routing is broken

rtk pnpm run test falls through to the generic "last N lines" fallback instead of routing through VitestParser. This is likely caused by stdout/stderr concatenation (format!("{}\n{}", stdout, stderr)) — pnpm writes boilerplate to stderr, vitest writes JSON to stdout. Concatenating them before passing to VitestParser::parse() breaks JSON extraction.

Fix: Apply filter_pnpm_run_output() to stderr only, pass stdout separately to the specialized filter.

2. is_pnpm_script() intercepts native pnpm subcommands

Reproduced: added "exec": "echo exec script" and "audit": "echo audit script" to package.json. Then:

• rtk pnpm exec vitest --version → runs the "exec" script instead of pnpm exec
• rtk pnpm audit → runs the "audit" script instead of pnpm audit

Fix: Add a denylist of known pnpm native subcommands (exec, dlx, create, patch, audit, why, link, unlink, rebuild, store, setup, init, publish, pack, config, doctor) in is_pnpm_script() before checking package.json.

3. ok ✓ with exit 1 outside a node project

rtk pnpm run test from a directory without package.json shows ok ✓ but exits 1. The boilerplate filter strips pnpm's error message, leaving a misleading success indicator for a failed command.

Fix: On non-zero exit, don't apply the boilerplate filter — show the raw error output.

Other items

• catch_unwind in apply_filter(): Replace with Result-based fallback — catch_unwind is for FFI/thread isolation, not application error handling
• Double package.json read: is_pnpm_script() reads it at dispatch time, then detect_tool_from_package_json() reads it again. Consolidate into one read.
• Token savings test at 40%: Project standard is 60% minimum
• extract_test_summary(output, "npm test") hardcodes wrong label: Will always trigger Jest-style parsing regardless of actual runner
• lazy_static! inside function: Move to module level to match RTK conventions

What works well

• Lint routing works correctly (tested with real ESLint errors)
• Build/dev script routing works
• Existing pnpm commands (list, outdated, install) are not broken
• Native pnpm subcommands (store prune, dlx, add) correctly passthrough
• pub(crate) promotions are minimal and appropriate
• Exit code preservation works on the filtered paths
• 437 tests pass, build compiles

Thanks for the solid work — keep it up! Happy to re-review once the vitest routing and subcommand interception are fixed.